Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ddl: check expr restriction for hash partitioned table #10273

Merged
merged 9 commits into from
May 8, 2019
Merged

ddl: check expr restriction for hash partitioned table #10273

merged 9 commits into from
May 8, 2019

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

Mysql:

mysql> CREATE TABLE `bb` (`id` int(11) NOT NULL,  
 `ts` TIMESTAMP(6) DEFAULT CURRENT_TIMESTAMP(6), 
 `c` char(120) NOT NULL, 
 KEY `k_ts` (`ts`) ) PARTITION BY   hash(ts);

ERROR 1659 (HY000): Field 'ts' is of a not allowed type for this type of partitioning

TiDB:
success

What is changed and how it works?

The return type of the expr should be int, and only part of functions are allowed
Ref:
https://dev.mysql.com/doc/refman/8.0/en/partitioning-limitations-functions.html
https://dev.mysql.com/doc/refman/8.0/en/partitioning-hash.html

Check List

Tests

  • Unit test

@tiancaiamao
ddl: check expr restriction for hash partitioned table
890e459 
The return type of the expr should be int, and only part of functions are allowed
@tiancaiamao
Copy link
Contributor Author

PTAL @crazycs520

@mahjonp
Copy link
Contributor

mahjonp commented Apr 26, 2019

/rebuild

@codecov
Copy link

codecov bot commented Apr 26, 2019 •
edited
Loading

Codecov Report

Merging #10273 into master will decrease coverage by 0.0085%.
The diff coverage is 70%.

@@               Coverage Diff                @@
##             master     #10273        +/-   ##
================================================
- Coverage   77.3592%   77.3506%   -0.0086%     
================================================
  Files           412        412                
  Lines         85717      85711         -6     
================================================
- Hits          66310      66298        -12     
- Misses        14373      14378         +5     
- Partials       5034       5035         +1

crazycs520
crazycs520 reviewed Apr 28, 2019
View reviewed changes
Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change https://github.com/pingcap/tidb/pull/10273/files#diff-07905132777c6fce0d33bf8f27a5dfaeR205 to

if s.Partition.Tp == model.PartitionTypeRange || s.Partition.Tp == model.PartitionTypeHash ?

@tiancaiamao tiancaiamao added the type/bugfix This PR fixes a bug. label Apr 29, 2019
@tiancaiamao
Copy link
Contributor Author

@crazycs520 Hash partition doesn't have "partition by columns ... "

@tiancaiamao tiancaiamao force-pushed the hash-partition-check branch from d63e1d3 to 46b9e00 Compare May 7, 2019 03:52
jackysp
jackysp reviewed May 7, 2019
View reviewed changes
Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

crazycs520
crazycs520 reviewed May 8, 2019
View reviewed changes
Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

crazycs520
crazycs520 reviewed May 8, 2019
View reviewed changes
ddl/ddl_api.go Outdated Show resolved Hide resolved
winkyao
winkyao approved these changes May 8, 2019
View reviewed changes
Copy link
Contributor

@winkyao winkyao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tiancaiamao tiancaiamao merged commit 54f3f3a into pingcap:master May 8, 2019
@tiancaiamao tiancaiamao deleted the hash-partition-check branch May 8, 2019 06:16
@tiancaiamao tiancaiamao mentioned this pull request May 8, 2019
Merged
@you06 you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crazycs520 crazycs520 crazycs520 left review comments

@jackysp jackysp jackysp left review comments

@winkyao winkyao winkyao approved these changes

@zimulala zimulala Awaiting requested review from zimulala

Assignees
No one assigned
Labels
sig/sql-infra SIG: SQL Infra type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tiancaiamao @mahjonp @jackysp @winkyao @crazycs520 @you06